Backend cpp for model-based semantic segmentation #33

DamienGilliard · 2024-06-24T13:09:46Z

In this branch we develop the backend of df cpp library to allow the model-based semantic segmentation.

…f joint detector function

…heck into semantic_segmentation

… segments to mesh faces

…ain branch

DamienGilliard · 2024-06-30T16:27:46Z

update:

The point association now uses face triangles instead of sampling points on the mesh pace and doing a p2p association. Works much better (the black points are the associated ones)
open3d is used for normal estimation instead of cilantro
current total computation time on point cloud of half a million points: 1400 ms

to do:

fix the reason why on some faces, a lot of points are excluded when they should be included
de-couple functions such as normal estimation, to make components more flexible (already discussed, not done yet)

…heck into semantic_segmentation

DamienGilliard · 2024-07-02T20:47:15Z

@9and3 I believe we can start the Ping-Pong (it took a bit longer than planned because a bird 🐦 flew through the window in the office, and getting it out became my mission. Birds don't know what's good for them...)

from top to bottom, the result of what this branch does: the rest of the clusters after the function is called, all the segments, with the selected points in black, and the selected point only.

9and3

@DamienGilliard first: I hope the little bird is fine.
On a second and less important matter, I left some comments. They are mainly generic and broad. Ping!

9and3 · 2024-07-04T20:27:59Z

src/diffCheck/segmentation/DFSegmentation.hh

+         * @param referenceMesh the vector of mesh faces to associate with the segments
+         * @param clusters the vector of clusters from cilantro to associate with the mesh faces of the reference mesh
+         * @param associationThreshold the threshold to consider the points of a segment and a mesh face as associable. The lower the number, the more strict the association will be and some poinnts on the mesh face might be wrongfully excluded.
+         * @return std::shared_ptr<geometry::DFPointCloud> The unified segments


has been added to the brief.
Do you think it should also be the name of the function, e.g. AssociateAndMergeClusters() ?

imo we can leave it as it is

9and3 · 2024-07-04T20:32:54Z

src/diffCheck/segmentation/DFSegmentation.hh

+         * @param referenceMesh the vector of mesh faces to associate with the segments
+         * @param clusters the vector of clusters from cilantro to associate with the mesh faces of the reference mesh
+         * @param associationThreshold the threshold to consider the points of a segment and a mesh face as associable. The lower the number, the more strict the association will be and some poinnts on the mesh face might be wrongfully excluded.
+         * @return std::shared_ptr<geometry::DFPointCloud> The unified segments


shouldn't we return a vector of DFPointClouds? So at the end we would have ideally as many as the meshes's vector length? Also what happened to your idea of returning the pointers of the DFPointCLouds that couldn't be asociated?

The clusters are taken as reference, as you suggested, so no need to return them.

My bad! Thanks for the clarification

9and3 · 2024-07-04T20:34:57Z

src/diffCheck/segmentation/DFSegmentation.hh

+        /** @brief Associates point cloud segments to mesh faces. It uses the center of mass of the segments and the mesh faces to find correspondances. For each mesh face it then iteratively associate the points of the segment that are actually on the mesh face.
+         * @param referenceMesh the vector of mesh faces to associate with the segments
+         * @param clusters the vector of clusters from cilantro to associate with the mesh faces of the reference mesh
+         * @param associationThreshold the threshold to consider the points of a segment and a mesh face as associable. The lower the number, the more strict the association will be and some poinnts on the mesh face might be wrongfully excluded.


is this a distance or angle what kind of value? Maybe you could add a distance but also normal threshold?
Might be worthy specifying it in the name and description.

The new description is: @param associationThreshold the threshold to consider the points of a segment and a mesh face as associable. It is the ratio between the surface of the closest mesh triangle and the sum of the areas of the three triangles that form the rest of the pyramid described by the mesh triangle and the point we want to associate or not. The lower the number, the more strict the association will be and some poinnts on the mesh face might be wrongfully excluded.

9and3 · 2024-07-04T20:44:08Z

src/diffCheck/segmentation/DFSegmentation.hh

+         * @param associationThreshold the threshold to consider the points of a segment and a mesh face as associable. The lower the number, the more strict the association will be and some poinnts on the mesh face might be wrongfully excluded.
+         * @return std::shared_ptr<geometry::DFPointCloud> The unified segments
+         */
+        static std::shared_ptr<geometry::DFPointCloud> DFSegmentation::AssociateClusters(


I know that we are trying to keep all very decoupled because of Grasshopper, but I still do not consider it as a "segmentation method". It's more of a "utility" or "segmentation refinement". Maybe we can add it in a section

public: ///< main segmentation methods static std::vector<std::shared_ptr<geometry::DFPointCloud>> NormalBasedSegmentation(...) public: ///< segmentation refiner static std::shared_ptr<geometry::DFPointCloud> DFSegmentation::AssociateClustersByMeshes(...)

Also, maybe we can specify better that we associate clusters with meshes.

9and3 · 2024-07-04T20:46:21Z

src/diffCheck/segmentation/DFSegmentation.cc

+            // Getting the normal of the mesh face
+            Eigen::Vector3d faceNormal = face->NormalsFace[0];


before running this we should make sure that meshes have normals, they might not have it. Let's just print an error message return an empty cloud and raise an error that meshes's should have normals

actually the comment was not correctly placed. the check was done a bit above. I have fixed the layout:

// Getting the normal of the mesh face if (face->NormalsFace.size() == 0) { o3dFace->ComputeTriangleNormals(); face->NormalsFace.clear(); for (auto normal : o3dFace->triangle_normals_) { face->NormalsFace.push_back(normal.cast<double>()); } } Eigen::Vector3d faceNormal = face->NormalsFace[0];

this function to compute normals would be worthy to be integrated in our DFMesh object class (@DamienGilliard )

9and3 · 2024-07-04T20:47:16Z

src/diffCheck/segmentation/DFSegmentation.cc

+
+            // Getting the center of the mesh face
+            Eigen::Vector3d faceCenter;
+            open3d::geometry::OrientedBoundingBox orientedBoundingBox = o3dFace->GetMinimalOrientedBoundingBox();


Having a OBB for getting the centroid of a mesh face seems a bit brittle, maybe a simpler mean of vertices would do the trick?

Moreover, if the o3dFace is in the x, y, or z plane, it is a OBB with volume = 0, which freaks out open3d. I have fixed this and use now the o3dFace->GetCenter() method (which I should have done from the beginning)

9and3 · 2024-07-04T20:49:54Z

src/diffCheck/segmentation/DFSegmentation.cc

+            // iterate through the segments to find the closest ones compared to the face center taking the normals into account
+            Eigen::Vector3d segmentCenter;
+            Eigen::Vector3d segmentNormal;
+            double faceDistance = std::numeric_limits<double>::max();
+            for (auto segment : clusters)
+            {
+                for (auto point : segment->Points)
+                {
+                    segmentCenter += point;
+                }
+                segmentCenter /= segment->GetNumPoints();
+
+                for (auto normal : segment->Normals)
+                {
+                    segmentNormal += normal;
+                }
+                segmentNormal.normalize();
+
+                double currentDistance = (faceCenter - segmentCenter).norm() / std::abs(segmentNormal.dot(faceNormal));
+                // if the distance is smaller than the previous one, update the distance and the corresponding segment
+                if (currentDistance < faceDistance)
+                {
+                    correspondingSegment = segment;
+                    faceDistance = currentDistance;
+                }
+            }
+
+            // get the triangles of the face.
+            std::vector<Eigen::Vector3i> faceTriangles = o3dFace->triangles_;
+
+            for (Eigen::Vector3d point : correspondingSegment->Points)
+            {
+                bool pointInFace = false;
+                /*
+                To check if the point is in the face, we take into account all the triangles forming the face.
+                We calculate the area of each triangle, then check if the sum of the areas of the tree triangles 
+                formed by two of the points of the referencr triangle and our point is equal to the reference triangle area 


just a thought here but in a future refinement having a ktree or some sort of structure for quick queries would highly improove this instead of looping through the cloud's points ?

Yes I aggree.

9and3 · 2024-07-04T20:53:58Z

btw, the results are starting to look fine! 🥇

DamienGilliard · 2024-07-04T21:08:46Z

Ok @9and3 , I'll check that tomorrow ! In the mean time, I have a refinement function, with promising results, for now. I'll do a quick wrap and see how it fares in Rhino, so we can discuss it tomorrow. The green part in the image is what is correctly added by the refinement function

… for cluster to mesh face association

…eneral cleanup

DamienGilliard · 2024-07-10T17:59:45Z

@9and3 I believe this PR is ready to be reviewed and merged if it's ok !
I have also added quite a few defensive checks

9and3

lgtm

9and3 and others added 22 commits June 2, 2024 22:28

FIX: the joint detector was not updated strange, now it's ok

ed64d49

FIX: cleaning needless prints

6b67948

FIX: for load mesh from file back to good version

4d7e015

WIP-FIX. got rid of pycache in gitignore

ad3a8cb

FIX: cleaning pycache directory

41da968

ADD: for box-like studs have a small catch in the detector

b585881

ADD: OBB added for both dfmesh and dfcloud + bindings

8331e35

WIP-CAP: working version for new joint detector

d7349fb

WIP: almost there, need packaging the new algorithm in the itnernal d…

41bea3f

…f joint detector function

WIP-CAP: need cleaning but the new joint detector is in place

029c5d2

CAP: new joint detector + 3 components + xml export refactor

9e48a88

ADD: example file for assembly system

4255081

Merge branch 'fix_joint_detector'

394a621

WIP-MERGE: cleaning from merge diffcheck joint detector

b4e6e24

FIX: compiling fix in c++ solved

56dff2e

WIP: light readme cleaning

4030226

FIX: typo on mermaid plan

803fee2

WIP-ADD: AssociateSegments function created

f6fd4e9

FIX: renaming of cilantro segmentation wrap following review

1adc9de

Merge branch 'semantic_segmentation' of github.com:diffCheckOrg/diffC…

e320d64

…heck into semantic_segmentation

WIP: creation of the aggregatino function that associates point cloud…

8bff20f

… segments to mesh faces

FIX-MERGE: bringing the assembly/beam structure to the segmentation m…

58daa2a

…ain branch

9and3 changed the title ~~Semantic segmentation based on 3d model~~ Backend cpp for model-based semantic segmentation Jun 29, 2024

9and3 assigned DamienGilliard Jun 29, 2024

9and3 added the dev label Jun 29, 2024

9and3 mentioned this pull request Jun 29, 2024

Frontend segmentation components #23

Merged

2 tasks

9and3 added 4 commits June 29, 2024 16:36

ADD: small cmake utilitiy for dev convinience

e0b531d

ADD: normal estimation to dfcloud

3940e23

TEMP: working on colored cluster + visualizer

acc643c

WIP: adding coloring for cloud and clustering

08b09db

9and3 and others added 3 commits June 30, 2024 18:12

CAP: refactored backend for segmeentations and normal estimations

3eb0fc5

WIP: segmentation now using triangle faces + small fixes

c9277c7

ADD: egg link to gitignore

2010cb0

9and3 and others added 6 commits July 1, 2024 00:06

ADD-MILESTONE: wraps for normal estimation + segmentation

73888c9

commit before rebase

af0c8b8

WIP: commit before pull

b24e67f

WIP: rebased onto 'segmentation' branch

50cdc17

Merge branch 'semantic_segmentation' of github.com:diffCheckOrg/diffC…

2503cfb

…heck into semantic_segmentation

CAP: AssociateClusters function implemented in the back-end

c8e9ad5

9and3 reviewed Jul 4, 2024

View reviewed changes

DamienGilliard added 4 commits July 6, 2024 20:11

UPDATE: CleanUnassociatedClusters method added, and PR remarks included

032f428

UPDATE-WIP: add IsPointOnFace function, and angle threshold parameter…

b688ec9

… for cluster to mesh face association

UPDATE: adding some preventive checks to the methods in DFSegmentation

111c484

CAP: Association functions implemented, defensive checks added, and g…

38c12c5

…eneral cleanup

9and3 approved these changes Jul 11, 2024

View reviewed changes

9and3 merged commit 060464a into segmentation Jul 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backend cpp for model-based semantic segmentation #33

Backend cpp for model-based semantic segmentation #33

DamienGilliard commented Jun 24, 2024 •

edited by 9and3

Loading

DamienGilliard commented Jun 30, 2024 •

edited

Loading

DamienGilliard commented Jul 2, 2024 •

edited

Loading

9and3 left a comment

9and3 Jul 4, 2024

DamienGilliard Jul 6, 2024

9and3 Jul 6, 2024

9and3 Jul 4, 2024

DamienGilliard Jul 6, 2024

9and3 Jul 6, 2024

9and3 Jul 4, 2024

DamienGilliard Jul 6, 2024

9and3 Jul 4, 2024

DamienGilliard Jul 6, 2024

9and3 Jul 4, 2024

DamienGilliard Jul 6, 2024

9and3 Jul 6, 2024 •

edited

Loading

DamienGilliard Jul 6, 2024

9and3 Jul 4, 2024

DamienGilliard Jul 6, 2024

9and3 Jul 4, 2024

DamienGilliard Jul 6, 2024

9and3 commented Jul 4, 2024

DamienGilliard commented Jul 4, 2024 •

edited

Loading

DamienGilliard commented Jul 10, 2024

9and3 left a comment

		// Getting the normal of the mesh face
		Eigen::Vector3d faceNormal = face->NormalsFace[0];

Backend cpp for model-based semantic segmentation #33

Backend cpp for model-based semantic segmentation #33

Conversation

DamienGilliard commented Jun 24, 2024 • edited by 9and3 Loading

DamienGilliard commented Jun 30, 2024 • edited Loading

DamienGilliard commented Jul 2, 2024 • edited Loading

9and3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9and3 Jul 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9and3 commented Jul 4, 2024

DamienGilliard commented Jul 4, 2024 • edited Loading

DamienGilliard commented Jul 10, 2024

9and3 left a comment

Choose a reason for hiding this comment

DamienGilliard commented Jun 24, 2024 •

edited by 9and3

Loading

DamienGilliard commented Jun 30, 2024 •

edited

Loading

DamienGilliard commented Jul 2, 2024 •

edited

Loading

9and3 Jul 6, 2024 •

edited

Loading

DamienGilliard commented Jul 4, 2024 •

edited

Loading